Rewards Greaselion - Twitter #6671

emerick · 2020-09-21T16:39:56Z

Resolves brave/brave-browser#11462
Resolves brave/brave-browser#6762
Resolves brave/brave-browser#5685
Resolves brave/brave-browser#4557
Resolves brave/brave-browser#8265

Associated Greaselion script PR: brave/brave-site-specific-scripts#16

Submitter Checklist:

Test Plan:

Note: In order to test this PR, you must manually install the Twitter Greaselion script at the moment. This involves pulling down this branch and running the following commands (the example is Windows specific, it will need to be modified slightly for Mac/Linux):

npm run build
cp -R dist/Greaselion.json /c/Users/$USERNAME/AppData/Local/BraveSoftware/Brave-Browser/User\ Data/afalakplffnnnlkncjhbmahjfjhmlkal/1.0.28/1/
cp -R dist/scripts /c/Users/$USERNAME/AppData/Local/BraveSoftware/Brave-Browser/User\ Data/afalakplffnnnlkncjhbmahjfjhmlkal/1.0.28/1/

Desktop

Standard URL

Clean profile
Run browser
Install Twitter Greaselion script as explained above
Restart browser
Visit a Twitter URL (e.g., https://twitter.com/emerick)
Verify that correct publisher information appears in Rewards panel
Verify that tipping works as expected
Verify that verified checkmark appears in Rewards icon for verified publishers

Excluded URL

Clean profile
Run browser
Install Twitter Greaselion script as explained above
Restart browser
Visit an excluded Twitter URL (e.g., https://twitter.com/settings)
Verify that correct publisher information appears in Rewards panel (twitter.com)
Verify that tipping works as expected

Activity

Clean profile
Run browser
Install Twitter Greaselion script as explained above
Restart browser
Visit a Twitter URL (e.g., https://twitter.com/emerick)
Verify that activity_info database table is updated as expected
Verify that auto-contribute table is updated to reflect new activity

History

Navigate to various Twitter URLs in the same tab
Verify that appropriate publisher information appears in the panel
Verify that back/forward navigation updates the panel with correct publisher info

Tab Activation

Navigate to various Twitter URLs in multiple tabs
Verify that appropriate publisher information appears in the panel
Verify that switching between tabs updates the panel with correct publisher info

Tipping non-Twitter Publishers

Verify that tipping works as expected on non-Twitter publishers (we only made changes to Twitter and only on desktop)
Verify that the panel looks as expected on non-Twitter publishers

Mobile

No changes were made to mobile platforms, so verify that Twitter navigation / tipping / Rewards panel continue to work as expected on those platforms.

Reviewer Checklist:

New files have MPL-2.0 license header.
Request a security/privacy review as needed.
Adequate test coverage exists to prevent regressions
Verify test plan is specified in PR before merging to source

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on.
All relevant documentation has been updated.

browser/extensions/api/brave_rewards_api.cc

vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.cc

zenparsing · 2020-09-28T15:57:50Z

components/brave_rewards/resources/extension/brave_rewards/background/publisherVisit.ts

+          const data = msg.data as MakeAPIRequest
+          handleMakeAPIRequest(data, (response: any) => {
+            port.postMessage({
+              type: 'MakeAPIRequest',


Looks like we're using the same type string for both the request and the response. Do you think it would be more clear to use different names (e.g. OnAPIRequest, OnAPIResponse, etc.)

I had Request and Response appended to all message types for a while when writing the YouTube script. I feel like it didn't really help with clarity and just made things wordier. Once the background script began making requests, it only added to the confusion (at least for me!) I'm going to try to stick with this nomenclature for consistency right now.

zenparsing · 2020-09-28T16:04:26Z

components/brave_rewards/resources/extension/brave_rewards/background/publisherVisit.ts

+      return response.json()
+    })
+    .then(responseData => sendResponse(responseData))
+    .catch(error => sendResponse(error))


It looks like we're sending both errors and valid responses through the "response" field into the content script. Does the content script know how to tell the difference? Perhaps the message to the content script should include both "response" and "error" fields?

Great point, fixed.

NejcZdovc

verified publisher doesn't have profile image https://twitter.com/lukemulks/status/1293113074317049856
should show publisher info https://twitter.com/brave/likes
current:

expected:
inline tip is not showing correct publisher. It should show TNW Events, but it shows Geertje as it was cited by TNW
https://twitter.com/tnwevents/status/1311659702598668288
current:

expected:

tmancey

LGTM++

emerick · 2020-10-05T20:50:08Z

All CI platforms passed, waiting for final approval on brave/brave-site-specific-scripts#16 before merging.

johntringham · 2020-12-16T10:44:16Z

I believe this change has caused an issue where the embedded tip button can't be turned off. There was a test to check whether disabling Rewards also disabled the Tip button, but this PR includes a change that disables it in this commit.

I might be wrong about this (first time looking at the codebase) but it looks like this PR rewrites the way the embedded tip buttons are created/rendered, but doesn't include any reference to the "props.rewardsData.inlineTip" preference, and so will be on by default no matter what the preference is set to.

emerick · 2020-12-16T14:07:24Z

@johntringham After you disable the tip button, it requires you to restart the browser (we're working on removing that limitation). If you do that, is the button disabled for you?

johntringham · 2020-12-17T16:40:05Z

Yep, I've tried that (and several other things) - the Tip icon still appears. It looks like this bug is there for Twitter, Github and Reddit.

This has been raised in several places over the last couple of months:
https://community.brave.com/t/no-way-of-removing-tip-button-from-twitter/188046/4
https://community.brave.com/t/twitter-showing-bat-tip-button-when-turned-off-causing-glitching/168542
brave/brave-browser#13223
brave/brave-browser#13222
brave/brave-browser#13157
brave/brave-browser#12870

Edit: Apologies - restarting the browser did fix it! I thought I had tried that but it looks like I had a couple of windows open in a different desktop, so the brave processes didn't fully end. Sorry about that - I'll stop posting on old PR's now :)

emerick · 2020-12-17T16:55:23Z

@johntringham No worries at all, glad it's working.

emerick added the feature/rewards label Sep 21, 2020

emerick requested a review from NejcZdovc as a code owner September 21, 2020 16:39

emerick self-assigned this Sep 21, 2020

emerick changed the title ~~Support inline tipping in Twitter via Greaselion~~ Rewards Greaselion - Twitter Sep 21, 2020

emerick marked this pull request as draft September 21, 2020 16:41

emerick force-pushed the rewards-greaselion-twitter branch 3 times, most recently from 72fcb96 to 6358b07 Compare September 24, 2020 02:20

NejcZdovc mentioned this pull request Sep 24, 2020

Add support for Rewards Twitter publishers brave/brave-site-specific-scripts#16

Merged

emerick force-pushed the rewards-greaselion-twitter branch 2 times, most recently from 76f156c to 84c283f Compare September 24, 2020 18:49

emerick requested a review from zenparsing September 24, 2020 23:45

emerick marked this pull request as ready for review September 24, 2020 23:45

emerick requested a review from tmancey as a code owner September 24, 2020 23:45

emerick force-pushed the rewards-greaselion-twitter branch 2 times, most recently from 8645f8d to e28499a Compare September 25, 2020 23:12

NejcZdovc reviewed Sep 28, 2020

View reviewed changes

browser/extensions/api/brave_rewards_api.cc Outdated Show resolved Hide resolved

NejcZdovc reviewed Sep 28, 2020

View reviewed changes

vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.cc Show resolved Hide resolved

emerick force-pushed the rewards-greaselion-twitter branch 2 times, most recently from f319632 to c9d366b Compare September 28, 2020 13:55

emerick requested a review from NejcZdovc September 28, 2020 13:56

emerick force-pushed the rewards-greaselion-twitter branch from c9d366b to 3ccbeb2 Compare September 28, 2020 15:58

zenparsing reviewed Sep 28, 2020

View reviewed changes

emerick force-pushed the rewards-greaselion-twitter branch 3 times, most recently from 3402f4c to cffc880 Compare September 28, 2020 18:17

emerick requested a review from zenparsing September 28, 2020 23:34

zenparsing approved these changes Sep 29, 2020

View reviewed changes

emerick force-pushed the rewards-greaselion-twitter branch 2 times, most recently from 6affe6a to 35cd56c Compare September 29, 2020 21:07

emerick force-pushed the rewards-greaselion-twitter branch 3 times, most recently from 67f651f to cfa337d Compare October 2, 2020 03:23

NejcZdovc suggested changes Oct 2, 2020

View reviewed changes

tmancey approved these changes Oct 2, 2020

View reviewed changes

emerick requested a review from NejcZdovc October 2, 2020 15:47

emerick force-pushed the rewards-greaselion-twitter branch 2 times, most recently from 4551fd4 to 5285464 Compare October 5, 2020 16:08

NejcZdovc approved these changes Oct 5, 2020

View reviewed changes

emerick added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Oct 5, 2020

emerick added 3 commits October 5, 2020 14:29

Support inline tipping in Twitter via Greaselion

1903d54

Show banner logo in Rewards panel if specified

4775ce9

TipUser needs to send favicon url

65aa51e

emerick force-pushed the rewards-greaselion-twitter branch from 5285464 to 65aa51e Compare October 5, 2020 18:30

emerick removed CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Oct 5, 2020

emerick merged commit 49eef4f into master Oct 5, 2020

emerick deleted the rewards-greaselion-twitter branch October 5, 2020 22:00

emerick added this to the 1.17.x - Nightly milestone Oct 5, 2020

LaurenWags mentioned this pull request Oct 13, 2020

[Desktop] Use Greaselion for Twitter publishers brave/brave-browser#11462

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewards Greaselion - Twitter #6671

Rewards Greaselion - Twitter #6671

emerick commented Sep 21, 2020 •

edited

Loading

zenparsing Sep 28, 2020

emerick Sep 28, 2020

zenparsing Sep 28, 2020

emerick Sep 28, 2020

NejcZdovc left a comment

tmancey left a comment

emerick commented Oct 5, 2020

johntringham commented Dec 16, 2020 •

edited

Loading

emerick commented Dec 16, 2020

johntringham commented Dec 17, 2020 •

edited

Loading

emerick commented Dec 17, 2020

Rewards Greaselion - Twitter #6671

Rewards Greaselion - Twitter #6671

Conversation

emerick commented Sep 21, 2020 • edited Loading

Submitter Checklist:

Test Plan:

Desktop

Standard URL

Excluded URL

Activity

History

Tab Activation

Tipping non-Twitter Publishers

Mobile

Reviewer Checklist:

After-merge Checklist:

zenparsing Sep 28, 2020

Choose a reason for hiding this comment

emerick Sep 28, 2020

Choose a reason for hiding this comment

zenparsing Sep 28, 2020

Choose a reason for hiding this comment

emerick Sep 28, 2020

Choose a reason for hiding this comment

NejcZdovc left a comment

Choose a reason for hiding this comment

tmancey left a comment

Choose a reason for hiding this comment

emerick commented Oct 5, 2020

johntringham commented Dec 16, 2020 • edited Loading

emerick commented Dec 16, 2020

johntringham commented Dec 17, 2020 • edited Loading

emerick commented Dec 17, 2020

emerick commented Sep 21, 2020 •

edited

Loading

johntringham commented Dec 16, 2020 •

edited

Loading

johntringham commented Dec 17, 2020 •

edited

Loading